feat(hyva): make the Banner Slider work on Hyvä (JS + CSS)#1
Open
ayeshanisar786 wants to merge 2 commits into
Open
feat(hyva): make the Banner Slider work on Hyvä (JS + CSS)#1ayeshanisar786 wants to merge 2 commits into
ayeshanisar786 wants to merge 2 commits into
Conversation
The slider only worked on Luma. Two Hyvä gaps: - JS: the slider inits via text/x-magento-init -> requireJS (slider.js). Hyvä strips requireJS + x-magento-init, so it never initialised. - CSS: styles ship as _module.less (Luma pipeline only). Hyvä does not compile Magento LESS, so the slider had NO CSS (slides rendered position:static, opacity:1 — stacked and unstyled). Fix (in slider.phtml, no new files, works on both themes): - Inline <style id=ef-bs-hyva-css> with the compiled-equivalent slider CSS. - A guarded vanilla controller that runs ONLY when window.require is absent (Hyvä); Luma still uses the existing x-magento-init/requireJS path. It reuses the same markup + .is-active crossfade and honours the admin settings (autoplay, speed, loop, arrows, bullets, pause-on-hover). Verified live on both themes (Hyvä + Luma): 3 banners crossfade-rotate, arrows/bullets work, autoplay + pause-on-hover correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The requireJS slider.js normally un-lazies each slide image (lazyLoad). On Hyvä that never runs, so slides 2..n kept their loading=lazy / data-src placeholder inside a visibility:hidden slide and rendered blank. Add loadSlide(): swap data-src->src, data-srcset->srcset, and force loading=eager (re-assigning src) for the shown slide + preload the next. Verified on the demo: all 3 banner images load and rotate on Hyvä. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The slider only worked on Luma — on Hyvä it never animated (only the first, unstyled slide showed). Two Hyvä gaps, both fixed in
slider.phtml(no new files):Root cause
text/x-magento-init→ requireJS (ETechFlow_BannerSlider/js/slider). Hyvä strips requireJS +x-magento-init, so it never initialised (no rotation, no arrows/bullets)._module.less, compiled only by Magentoʼs (Luma) LESS pipeline. Hyvä doesnʼt compile Magento LESS, so the slider had zero CSS — slides renderedposition:static; opacity:1, stacked and unstyled.Fix
<style id="ef-bs-hyva-css">with the compiled-equivalent slider CSS (viewportaspect-ratio, absolute stacked slides,.is-activecrossfade, effect-slide, arrows, bullets, mobile). Identical rules → harmless duplicate on Luma.window.requireis absent (i.e. Hyvä); Luma still uses the existingx-magento-init/requireJS path. It reuses the same markup +.is-activecrossfade and honours the admin settings: autoplay, speed, loop, arrows, bullets, pause-on-hover.Verified
Tested live on the demo store on both themes via browser automation — 3 banners crossfade-rotate
1→2→0→1, arrows/bullets work, autoplay + pause-on-hover correct.Notes / follow-ups
targeting.js, customer-data section) and click tracking (tracker.js) which are likewise requireJS-only today — out of scope here (basic slider parity first).<style>is emitted per rendered slider; if multiple sliders appear on one page it could be guarded to emit once. Minor.🤖 Generated with Claude Code